-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Enable Shard Search-load rate metrics for injection #127743
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable Shard Search-load rate metrics for injection #127743
Conversation
|
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
server/src/main/java/org/elasticsearch/index/search/stats/ShardSearchLoadRateService.java
Outdated
Show resolved
Hide resolved
piergm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, I wonder if we should ask for another pair of eyes on this before merging. Otherwise you can merge it as long as the CI is happy :D
Definitely, thank you, Matteo for the review! |
andreidan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this Dimi.
I have some initial questions regarding the overall design.
Also, please update the PR description to contain more details around the what and the why for this effort.
server/src/main/java/org/elasticsearch/index/search/stats/SearchStatsSettings.java
Outdated
Show resolved
Hide resolved
| CollectionUtils.appendToCopyNoNullElements(listeners, internalIndexingStats, indexingFailuresDebugListener), | ||
| logger | ||
| ); | ||
| this.shardSearchLoadRateService = shardSearchLoadRateProvider.create(searchStatsSettings, System::currentTimeMillis); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why we're injecting a service here. Could the index shard calculate the ewmr directly?
What's the reason for the SPI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good question. The main reason for injecting the service via SPI rather than calculating the EWMR directly in IndexShard is to avoid unnecessary memory overhead when the functionality isn't needed. While lazy initialization behind a proxy is an option on the stateful side, it would add unnecessary complexity to the core codebase.
Additionally, this design helps separate concerns: the implementation is specific to the multi-project setup and doesn't belong in the core logic of IndexShard. Placing it behind an SPI keeps the core codebase clean and aligns the implementation more closely with the project that owns the logic, improving maintainability and modularity. So, keeping it behind an SPI ensures it's only enabled where applicable.
server/src/main/java/org/elasticsearch/index/search/stats/SearchStatsSettings.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/search/stats/SearchStatsSettings.java
Outdated
Show resolved
Hide resolved
|
This pr has been closed in favor of #128660 |
This work presents an approach for tracking search load per index in a multi-project Elasticsearch environment, with the goal of dynamically adjusting replica counts to optimize resource utilization and maintain a balance between under- and overutilization.
The implementation integrates
ShardSearchLoadRateServiceintoIndexShardas an SPI extension point, enabling flexible extension of shard-level search load tracking.Additionally, it introduces the
SearchStatsSettingsclass, which enables half-life calculations withinStatelessShardSearchLoadRateService::ExponentiallyWeightedMovingRate, and facilitates scheduled, pull-based collection of search node metrics in serverless deployments.